-
Notifications
You must be signed in to change notification settings - Fork 539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[swssconfig] Optimize performance of swssconfig #2336
[swssconfig] Optimize performance of swssconfig #2336
Conversation
This pull request introduces 1 alert when merging 556d703 into 700492f - view on LGTM.com new alerts:
|
swssconfig/swssconfig.cpp
Outdated
SWSS_LOG_ERROR("Invalid operation: %s\n", kfvOp(db_item).c_str()); | ||
return false; | ||
} | ||
} | ||
|
||
flush_db_data(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, I want to keep the logic as same as before. Before the change, all data will be put into redis DB until a failure occurs. So, here I call flush_db_data()
at 3 places (2 return false, 1 return true) to make sure the data will be flushed into redis even if a failure happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understand your point. The pattern here is a python finally
like syntax to achieve cleanup if out of scope. C++ has some best practice like RAII to achieve this.
Actually in this case, if you limit table_map
scope to this function, not as a global function. The dtor will automatically flush all pipelines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose I understand your point. RedisPipleLine will flush all data in its dtor, so if pipeline is defined in this function, it will automatically do flush. I will make this change.
This pull request introduces 1 alert when merging ba88348 into 700492f - view on LGTM.com new alerts:
|
Could you fix LGTM alert? |
/azpw run Azure.sonic-swss |
/AzurePipelines run Azure.sonic-swss |
Azure Pipelines successfully started running 1 pipeline(s). |
It seems the checker is running a test case which is newer than my base code, I need to do a merge now. |
/azpw run Azure.sonic-swss |
/AzurePipelines run Azure.sonic-swss |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run Azure.sonic-swss |
Azure Pipelines successfully started running 1 pipeline(s). |
- What I did Optimize swssconfig: 1. Use unix socket 2. Cache producer table to avoid create it for same table name - Why I did it We found that generating large scale static routes via swssconfig is very slow. - How I verified it After the optimization, generating 100K routes via swssconfig take 2 seconds, however, before the optimization it takes > 60 seconds.
2022-06-24 93af69c: [PFC_WD] Avoid applying ZeroBuffer Profiles to ingress PG when a PFC storm is detected (sonic-net/sonic-swss#2304) 2022-06-24 37349cf: [swssconfig] Optimize performance of swssconfig (sonic-net/sonic-swss#2336) 2022-06-24 84e9b07: [fdborch] fix heap-use-after-free in clearFdbEntry() (sonic-net/sonic-swss#2353) 2022-06-24 1b8bd94: Create ACL table fails due to incorrect check for supported ACL actions #11235 (sonic-net/sonic-swss#2351) 2022-06-24 1ed0b4b: [macsec] Refactor the logic of macsec name map (sonic-net/sonic-swss#2348) 2022-06-23 f88f992: [mock_tests] Add Sflow Orch UTs (sonic-net/sonic-swss#2295) 2022-06-23 ec57bf1: [macsec] Update macsec flex counter (sonic-net/sonic-swss#2338) 2022-06-22 6e0fc85: [ACL] Support stage particular match fields (sonic-net/sonic-swss#2341) 2022-06-22 efb4530: [orchagent, DTel]: report session support to set user vrf (sonic-net/sonic-swss#2326) 2022-06-22 d82874d: Fix for "orchagent crashed when trying to delete fdb static entry with swssconfig #11046" (sonic-net/sonic-swss#2332) 2022-06-22 0c789e6: Fix qos map test in vs test (sonic-net/sonic-swss#2343) 2022-06-17 1bb5070: Enhance mock test for dynamic buffer manager for port removing and qos reload flows (sonic-net/sonic-swss#2262) 2022-06-16 700492f: [aclorch] Fix and simplify DTel watchlist tables and entries (sonic-net/sonic-swss#2155)
- What I did Optimize swssconfig: 1. Use unix socket 2. Cache producer table to avoid create it for same table name - Why I did it We found that generating large scale static routes via swssconfig is very slow. - How I verified it After the optimization, generating 100K routes via swssconfig take 2 seconds, however, before the optimization it takes > 60 seconds.
What I did
Optimize swssconfig:
Why I did it
We found that generating large scale static routes via swssconfig is very slow.
How I verified it
After the optimization, generating 100K routes via swssconfig take 2 seconds, however, before the optimization it takes > 60 seconds.
Details if related